I guess this returns null if it catches the exception?

        public static PageId forString(String name) {
            try {
                return PageIdNames.forString(name);
            } catch (Exception e) {
                throw new ICUException("No PageId for " + name, e);
            }
        }

that's a feature of Java I wasn't familiar with

no, that would throw.

Oh, right; I misread it. It catches IllegalArgumentException then throws ICUException.

So the removal of catch (Throwable t) means that searchPathheader may throw ICUException where before it wouldn't have. That goes beyond merely changing valueOf to forString. Here q comes from an http request, so it could be anything... Line 765 of SurveyAjax will catch it:

        } catch (Throwable e) {
            SurveyLog.logException(
                    logger, e, "Error in SurveyAjax?what=" + what + ": " + e.getMessage());
            sendError(out, e);
        }

I guess that's OK, though shouldn't searchPathheader then be declared to throw ICUException? It still silently catches other exceptions further below; it just seems a little arbitrary...

Oh, right; I misread it. It catches IllegalArgumentException then throws ICUException.

So the removal of catch (Throwable t) means that searchPathheader may throw ICUException where before it wouldn't have. That goes beyond merely changing valueOf to forString. Here q comes from an http request, so it could be anything... Line 765 of SurveyAjax will catch it:

        } catch (Throwable e) {
            SurveyLog.logException(
                    logger, e, "Error in SurveyAjax?what=" + what + ": " + e.getMessage());
            sendError(out, e);
        }

I guess that's OK, though shouldn't searchPathheader then be declared to throw ICUException? It still silently catches other exceptions further below; it just seems a little arbitrary...